Skip to content

refactor(api): centralize service-tier primitives - #1040

Merged
edelauna merged 14 commits into
Zoo-Code-Org:mainfrom
WebMad:refactor/service-tier-primitives
Jul 30, 2026
Merged

refactor(api): centralize service-tier primitives#1040
edelauna merged 14 commits into
Zoo-Code-Org:mainfrom
WebMad:refactor/service-tier-primitives

Conversation

@WebMad

@WebMad WebMad commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Responsibility and scope

Centralizes the shared service-tier primitives used by provider request construction so later stack layers can consume one canonical representation. Tests travel with the behavior in this PR.

Behavior and non-goals

  • Refactor only: there is no behavior change and no request-shape change.
  • Does not add OpenAI Codex Fast priority handling.
  • Does not add settings UI or translations.
  • Does not include the unrelated Task.throttle.test.ts fix.
  • Does not add a changeset.

Test evidence

  • Existing focused coverage was updated/retained with the refactored primitives.
  • Repository type checks passed from the push hook.
  • The completed stack was locally verified before publication.

Stack dependency

  1. This PR — shared service-tier primitives (base: canonical main).
  2. WebMad/Zoo-Code#2 — backend Fast priority persistence/request behavior.
  3. WebMad/Zoo-Code#3 — webview speed selector and translations.

Review and merge this PR first; each subsequent PR is based on the preceding contributor branch.

Summary by CodeRabbit

  • New Features
    • Standardized OpenAI service-tier handling for Default, Flex, and Priority, enabling consistent tier pricing display in model settings for supported tiers.
  • Bug Fixes
    • Fixed streamed and fallback usage/cost calculations to use the resolved tier (while keeping the requested tier in requests).
    • Improved service-tier request wiring for OpenAI and Bedrock, including correct omission when the tier isn’t supported.
  • Tests
    • Expanded coverage for OpenAI/Bedrock service-tier payloads, pricing selection, and UI table/selector behavior.
  • Chores
    • Updated package exports and Playwright translation test scaffolding.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

OpenAI service tiers are centralized in a shared enum and payload-key constant, then applied to provider request/response handling, pricing calculations, and settings UI components. Tests cover request payloads, resolved-tier pricing, streaming behavior, tier selection, and pricing-table rendering.

Changes

Service tier integration

Layer / File(s) Summary
Service tier contract
packages/types/src/model.ts, packages/types/package.json
Exports SERVICE_TIER_KEY and OpenAiServiceTier, derives service-tier values from the enum, and exposes model-related subpaths.
Provider requests, events, and pricing
src/api/providers/bedrock.ts, src/api/providers/openai-native.ts, src/shared/cost.ts, src/api/providers/__tests__/*, src/utils/__tests__/cost.spec.ts
Uses shared service-tier keys and enum values across Bedrock and OpenAI payloads, SSE event parsing, pricing defaults, and resolved-tier cost tests.
Service-tier settings and pricing UI
webview-ui/src/components/settings/ModelInfoView.tsx, webview-ui/src/components/settings/providers/OpenAI.tsx, webview-ui/src/components/settings/**/__tests__/*
Uses enum-based tier selection, renders tier-specific pricing with Standard fallbacks, and adds component coverage.
Settings test support and visual coverage
webview-ui/playwright/*, webview-ui/playwright-ct.config.ts, webview-ui/src/components/settings/__tests__/ModelInfoView.visual.*, webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx
Adds translation and fixture support, updates component-test aliases and imports, and adds a dark-theme visual regression test.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: awaiting-review

Suggested reviewers: taltas

Sequence Diagram(s)

sequenceDiagram
  participant OpenAiNativeHandler
  participant OpenAIResponsesAPI
  participant SSEEventProcessor
  participant CostCalculator
  OpenAiNativeHandler->>OpenAIResponsesAPI: Send request with service tier
  OpenAIResponsesAPI-->>SSEEventProcessor: Stream events with resolved tier
  SSEEventProcessor->>CostCalculator: Calculate cost from resolved tier
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning It doesn't follow the required template and omits the linked issue, test procedure, checklist, and other required sections. Add the template sections, especially Closes: #issue, Description, Test Procedure, and the pre-submission checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main refactor to centralize shared service-tier primitives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.84211% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ettings/__tests__/ModelInfoView.visual.fixture.tsx 0.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@WebMad
WebMad marked this pull request as ready for review July 29, 2026 00:51

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/api/providers/openai-native.ts (1)

373-376: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate tier-gating logic between buildRequestBody and completePrompt.

Both methods independently rebuild allowedTierNames and re-implement the identical "is tier allowed" condition (requestedTier === OpenAiServiceTier.Default || allowedTierNames.has(requestedTier)). Extracting a shared private helper avoids future divergence if the gating rule changes.

♻️ Proposed helper extraction
+	private resolveEffectiveServiceTier(model: OpenAiNativeModel): ServiceTier | undefined {
+		const requestedTier = (this.options.openAiNativeServiceTier as ServiceTier | undefined) || undefined
+		if (!requestedTier) return undefined
+		const allowedTierNames = new Set(model.info.tiers?.map((t) => t.name).filter(Boolean) || [])
+		return requestedTier === OpenAiServiceTier.Default || allowedTierNames.has(requestedTier)
+			? requestedTier
+			: undefined
+	}
+
 	private buildRequestBody(...): any {
 		...
-		const requestedTier = (this.options.openAiNativeServiceTier as ServiceTier | undefined) || undefined
-		const allowedTierNames = new Set(model.info.tiers?.map((t) => t.name).filter(Boolean) || [])
+		const effectiveRequestedTier = this.resolveEffectiveServiceTier(model)
 		...
-			...(requestedTier &&
-				(requestedTier === OpenAiServiceTier.Default || allowedTierNames.has(requestedTier)) && {
-					[SERVICE_TIER_KEY]: requestedTier,
-				}),
+			...(effectiveRequestedTier && { [SERVICE_TIER_KEY]: effectiveRequestedTier }),
 	async completePrompt(prompt: string, options?: CompletePromptOptions): Promise<string> {
 		...
-		const requestedTier = (this.options.openAiNativeServiceTier as ServiceTier | undefined) || undefined
-		const allowedTierNames = new Set(model.info.tiers?.map((t) => t.name).filter(Boolean) || [])
-		if (requestedTier && (requestedTier === OpenAiServiceTier.Default || allowedTierNames.has(requestedTier))) {
-			requestBody[SERVICE_TIER_KEY] = requestedTier
+		const effectiveRequestedTier = this.resolveEffectiveServiceTier(model)
+		if (effectiveRequestedTier) {
+			requestBody[SERVICE_TIER_KEY] = effectiveRequestedTier
 		}

Also applies to: 1513-1515

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/providers/openai-native.ts` around lines 373 - 376, Extract the
shared tier-approval logic from buildRequestBody and completePrompt into a
private helper that accepts requestedTier and the allowed tier names, then reuse
it in both SERVICE_TIER_KEY construction paths. Remove the duplicated
allowedTierNames setup and condition while preserving acceptance of
OpenAiServiceTier.Default and configured allowed tiers.
webview-ui/src/components/settings/ModelInfoView.tsx (1)

149-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Flex and Priority pricing rows are near-identical; extract a shared row renderer.

Each block repeats the same td/fmt/tiers.find(...) pattern, differing only by tier enum and label. A small helper removes the duplication and keeps the two rows from drifting.

♻️ Proposed row-renderer helper
+							{([
+								[OpenAiServiceTier.Flex, t("settings:serviceTier.flex")],
+								[OpenAiServiceTier.Priority, t("settings:serviceTier.priority")],
+							] as const)
+								.filter(([tierName]) => allowedTierNames.includes(tierName))
+								.map(([tierName, label]) => {
+									const tierInfo = modelInfo?.tiers?.find((t) => t.name === tierName)
+									return (
+										<tr key={tierName} className="border-t border-vscode-dropdown-border/60">
+											<td className="px-3 py-1.5">{label}</td>
+											<td className="px-3 py-1.5 text-right">
+												{fmt(tierInfo?.inputPrice ?? modelInfo?.inputPrice)}
+											</td>
+											<td className="px-3 py-1.5 text-right">
+												{fmt(tierInfo?.outputPrice ?? modelInfo?.outputPrice)}
+											</td>
+											<td className="px-3 py-1.5 text-right">
+												{fmt(tierInfo?.cacheReadsPrice ?? modelInfo?.cacheReadsPrice)}
+											</td>
+										</tr>
+									)
+								})}
-							{allowedTierNames.includes(OpenAiServiceTier.Flex) && ( ... )}
-							{allowedTierNames.includes(OpenAiServiceTier.Priority) && ( ... )}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@webview-ui/src/components/settings/ModelInfoView.tsx` around lines 149 - 194,
In ModelInfoView, extract the duplicated Flex and Priority pricing-row markup
into a shared row renderer or component that accepts the tier enum and
translated label, while preserving the existing allowedTierNames checks and
fallback pricing behavior. Replace both inline blocks with calls to this helper
so the tier-specific lookup and labels remain configurable without repeating the
td/fmt structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/api/providers/openai-native.ts`:
- Around line 373-376: Extract the shared tier-approval logic from
buildRequestBody and completePrompt into a private helper that accepts
requestedTier and the allowed tier names, then reuse it in both SERVICE_TIER_KEY
construction paths. Remove the duplicated allowedTierNames setup and condition
while preserving acceptance of OpenAiServiceTier.Default and configured allowed
tiers.

In `@webview-ui/src/components/settings/ModelInfoView.tsx`:
- Around line 149-194: In ModelInfoView, extract the duplicated Flex and
Priority pricing-row markup into a shared row renderer or component that accepts
the tier enum and translated label, while preserving the existing
allowedTierNames checks and fallback pricing behavior. Replace both inline
blocks with calls to this helper so the tier-specific lookup and labels remain
configurable without repeating the td/fmt structure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 56e7c7fe-047d-4f43-8288-851cb3b38ebe

📥 Commits

Reviewing files that changed from the base of the PR and between d27153a and 007a4bd.

📒 Files selected for processing (12)
  • packages/types/src/model.ts
  • src/api/providers/__tests__/bedrock.spec.ts
  • src/api/providers/__tests__/openai-native-usage.spec.ts
  • src/api/providers/__tests__/openai-native.spec.ts
  • src/api/providers/bedrock.ts
  • src/api/providers/openai-native.ts
  • src/shared/cost.ts
  • src/utils/__tests__/cost.spec.ts
  • webview-ui/src/components/settings/ModelInfoView.tsx
  • webview-ui/src/components/settings/__tests__/ModelInfoView.spec.tsx
  • webview-ui/src/components/settings/providers/OpenAI.tsx
  • webview-ui/src/components/settings/providers/__tests__/OpenAI.spec.tsx

@github-actions github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Jul 29, 2026

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for this PR - had 2 comments related to the implementation.

Comment thread packages/types/src/model.ts Outdated
Comment thread src/api/providers/__tests__/openai-native.spec.ts Outdated
@github-actions github-actions Bot added awaiting-author PR is waiting for the author to address requested changes and removed awaiting-review PR changes are ready and waiting for maintainer re-review labels Jul 29, 2026
@WebMad

WebMad commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both review comments in 5f0179c:

  • Replaced the TypeScript enum with an as const OpenAiServiceTier object, kept serviceTiers as a readonly tuple for z.enum, and now infer ServiceTier from the schema.
  • Reworked the three pricing-invariant tests to exercise the public createMessage usage path instead of reaching into private methods with Reflect.get / Reflect.apply.

Also centralized request-tier gating so streaming requests and completePrompt use the same helper, and kept the shared tier-pricing row renderer strictly typed with ServiceTier.

Validation completed:

  • 197 focused backend tests passed
  • 6 focused webview tests passed
  • repository lint and all 11 applicable type-check tasks passed via commit/push hooks

@WebMad
WebMad requested a review from edelauna July 29, 2026 15:36
@github-actions github-actions Bot added awaiting-review PR changes are ready and waiting for maintainer re-review and removed awaiting-author PR is waiting for the author to address requested changes labels Jul 29, 2026

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're touching the file could you include a snapshot of webview-ui/src/components/settings/ModelInfoView.tsx using https://github.com/Zoo-Code-Org/Zoo-Code/blob/main/webview-ui/AGENTS.md#visual-tests-playwright-ct

@github-actions github-actions Bot added awaiting-author PR is waiting for the author to address requested changes and removed awaiting-review PR changes are ready and waiting for maintainer re-review labels Jul 30, 2026
@WebMad

WebMad commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Since you're touching the file could you include a snapshot of webview-ui/src/components/settings/ModelInfoView.tsx using https://github.com/Zoo-Code-Org/Zoo-Code/blob/main/webview-ui/AGENTS.md#visual-tests-playwright-ct

Anything you want, bro 😄

@github-actions github-actions Bot removed the awaiting-author PR is waiting for the author to address requested changes label Jul 30, 2026
@WebMad
WebMad requested a review from edelauna July 30, 2026 10:17

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/types/package.json`:
- Around line 14-21: Update the package export entries for the “./model” and
“./provider-identifiers” subpaths to support CommonJS consistently with the root
export: add matching require targets that point to their generated CommonJS
build outputs, or explicitly declare both subpaths ESM-only if that is the
intended contract. Ensure the selected approach prevents require() from
producing ERR_PACKAGE_PATH_NOT_EXPORTED.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 85793c52-1c10-42f1-8d66-b1c44e6e66ac

📥 Commits

Reviewing files that changed from the base of the PR and between a92aed6 and cda1af2.

📒 Files selected for processing (6)
  • packages/types/package.json
  • webview-ui/playwright-ct.config.ts
  • webview-ui/playwright/TranslationContext.ts
  • webview-ui/src/components/settings/ModelDescriptionMarkdown.tsx
  • webview-ui/src/components/settings/ModelInfoView.tsx
  • webview-ui/src/components/settings/__tests__/ModelInfoView.visual.fixture.tsx

Comment thread packages/types/package.json
@github-actions github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Jul 30, 2026

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the snapshots

@edelauna
edelauna added this pull request to the merge queue Jul 30, 2026
Merged via the queue into Zoo-Code-Org:main with commit 569b43d Jul 30, 2026
18 checks passed
xavier-arosemena added a commit to xavier-arosemena/roo-plus that referenced this pull request Aug 3, 2026
…tocol, slim ClineProvider, upstream sync (#112)

* refactor(webview): use canonical provider identifiers (Zoo-Code-Org#1023)

* refactor(webview): use canonical provider identifiers

* test(webview): remove brittle identifier mutation tests

* fix(webview): handle remaining model providers explicitly

* test(webview): avoid unreachable coverage branch

* refactor(webview): use canonical anthropic identifier

* test(webview): cover Anthropic Opus 1M tier

* test(webview): use canonical Anthropic identifier

* fix(webview): address canonical provider review

* test(webview): use canonical provider identifiers

* test(webview): cover Kimi Code model selection

* refactor: finish canonical provider identifier audit (Zoo-Code-Org#1030)

* refactor: finish canonical provider identifier audit

* fix: address provider identifier review feedback

* refactor: centralize Anthropic protocol value

* refactor: reuse canonical provider protocol constants

* refactor: centralize OpenAI protocol value

* test: use canonical keyless provider identifiers

* test: address protocol routing review feedback

* refactor(api): centralize service-tier primitives (Zoo-Code-Org#1040)

* refactor(api): centralize service-tier primitives

* test(openai-native): cover omitted completion tiers

* test(openai-native): cover resolved streaming tiers

* test(api): cover service tier fallbacks

* test(api): cover remaining service tier branches

* refactor(api): remove duplicate tier capture

* test(openai-native): cover flex service tier

* test(openai-native): expand service tier coverage

* refactor: address service tier review feedback

* refactor(api): address service tier review feedback

* test(webview): add model info visual snapshot

* fix(webview): isolate visual test dependencies

* test(webview): update service tier visual baseline

* fix(webview): add @source directive so Tailwind utilities generate in CT builds

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>

* fix(openai-compatible): add max reasoning effort option (Zoo-Code-Org#882) (Zoo-Code-Org#1051)

* feat(openai-codex): persist and send Fast priority mode (Zoo-Code-Org#1063)

* feat(openai-codex): persist and send Fast priority mode

* refactor(openai-codex): align service tier types

* fix(readiness): remove stale cloud test, fix floating promises, rebrand webview title

* chore: bump custom-modes submodule (user-friendly descriptions) + add roomodes sync guard

- Point custom-modes at 029a0d4 (description rewrite + YAML normalization)
- Make sync-custom-modes.mjs importable and add scripts/verify-roomodes-sync.mjs
  which fails when .roomodes is not reproducible from the submodule

* feat(protocol): add zod message registry + boundary validation (S1-M1/M2)

* feat(protocol): type + validate allowedCommands/deniedCommands (S1-M3 domain 1)

- Add commands.ts schemas registered in webviewMessageSchemas (allowedCommands, deniedCommands require a string[]).
- Migrate the handler cases to schema-validated typed payloads, dropping the runtime Array.isArray/typeof sanitization now guaranteed by zod.
- Boundary now rejects crafted non-array commands payloads (unit + handler + ClineProvider boundary tests).

* feat(protocol): type + validate updateSettings (S1-M3 domain 2)

- rooCodeSettingsSchema now uses .passthrough() so unknown future settings fields are retained, not stripped.
- Add settings.ts updateSettings schema (updatedSettings?: RooCodeSettings) registered in webviewMessageSchemas.
- Migrate the updateSettings handler case to the schema-validated payload; malformed known-field types (e.g. non-string terminalProfile) are rejected before side effects.
- Boundary + handler tests cover valid dispatch and malformed rejection.

* feat(protocol): type + validate provider config messages (S1-M3 domain 3)

- Add providerConfig.ts schemas for saveApiConfiguration, upsertApiConfiguration (text + ProviderSettings-passthrough) and setApiConfigPassword, registered in webviewMessageSchemas.
- apiConfiguration reuses providerSettingsSchema.passthrough(): key fields (incl. apiProvider enum) validated, provider-specific passthrough fields retained.
- Migrate save/upsert handler cases to schema-validated typed payloads, dropping the runtime text/apiConfiguration guards.
- setApiConfigPassword remains a no-op but malformed shapes are now rejected at the boundary.

* feat(protocol): type + validate marketplace install messages (S1-M3 domain 4)

- Add marketplace.ts schemas for installMarketplaceItem, installMarketplaceItems (min 1 item) and installMarketplaceItemWithParameters, registered in webviewMessageSchemas.
- mpItem/mpItems reuse marketplaceItemSchema; mpInstallOptions reuses installMarketplaceItemOptionsSchema.
- Migrate the three handler cases to schema-validated typed payloads; crafted non-item payloads are rejected at the boundary.

* feat(protocol): type + validate chat message queue messages (S1-M3 domain 5)

- Add messageQueue.ts schemas for queueMessage (text + images), removeQueuedMessage (text) and editQueuedMessage (payload reused from queuedMessageSchema.pick), registered in webviewMessageSchemas.
- Migrate the three handler cases to schema-validated typed payloads, removing the payload-as-EditQueuedMessagePayload cast and the now-unused import.
- Boundary + handler tests cover valid dispatch and malformed rejection (non-string text).

* feat(protocol): type + validate todo list and custom mode messages (S1-M3 domain 6)

- Add customModes.ts schemas for updateTodoList (payload.todos from todoItemSchema), updateCustomMode (slug + modeConfig) and deleteCustomMode (slug + optional checkOnly), registered in webviewMessageSchemas.
- Migrate the three handler cases to schema-validated typed payloads; updateTodoList drops the payload-as-any cast (handler no-explicit-any suppression 5 -> 4).
- Fix the ClineProvider updateCustomMode boundary test to include the top-level slug, matching the real webview sender contract (do not loosen the schema).
- Boundary + handler tests cover valid dispatch and malformed rejection.

* test(protocol): guard updateSettings sender compatibility

Add parseWebviewMessage assertions for the CLI extension-host initialSettings
shape and the webview SettingsView handleSubmit payload (incl. nullable/edge
fields) so the updateSettings schema never regresses the real senders.

* ci: add message-schema ratchet guard (S1-M4)

* refactor(webview): extract shared handler helpers and narrow provider types (S2 scaffold)

- Add handlers/shared.ts with getGlobalState/updateGlobalState/getCurrentCwd/resolveIncomingImages
  extracted verbatim from webviewMessageHandler's pre-switch setup.
- Narrow provider params in checkpointRestoreHandler, generateSystemPrompt, worktree/handlers,
  skillsMessageHandler, rulesMessageHandler to minimal Pick<ClineProvider, ...> types so domain
  handlers depend only on the members they use (dependency inversion; ClineProvider satisfies
  each Pick structurally, so callers need no cast).
- Exempt core/webview/handlers/*.ts from no-case-declarations (same rule the original dispatcher
  used for its switch case bodies).
- Re-home the 4 no-explicit-any suppressions from webviewMessageHandler.ts to handlers/chat.ts
  (the moved casts keep their existing suppressions).

No behavior change: moved helper bodies are identical and the old dispatcher still compiles
against the narrowed signatures.

* refactor(webview): add per-domain webview message handler modules (S2)

Move every case from webviewMessageHandler's giant switch into domain modules under
core/webview/handlers/, each exporting a ReadonlySet<WebviewMessageType> of the types it
handles plus a handle<Domain>Messages(provider, marketplaceManager, message) function that
switch-dispatches those cases VERBATIM (same bodies, ordering, and error handling).

  chat.ts            17 types (message edit/delete/confirm, queue, tts, checkpoints, enhancePrompt)
  task.ts            17 types (new/clear/cancel/condense/export/delete tasks, system prompt, commits, todos)
  settings.ts        27 types (updateSettings, allowed/deniedCommands, custom modes, prompts, models)
  providerProfiles.ts 15 types (api config CRUD, pins, provider OAuth sign-in/out, rate limits)
  mcp.ts              9 types (server lifecycle, tool toggles, timeout, settings)
  marketplace.ts      7 types (install/remove/filter/fetch, mdm notification)
  worktree.ts        11 types (list/create/delete/switch worktrees, branches, includes, picker)
  codeIndex.ts        8 types (settings, indexing lifecycle, secrets, auto-enable)
  skills.ts           6 types   rules.ts 5 types   commands.ts 4 types
  terminal.ts         3 types   images.ts 3 types   debug.ts   3 types
  misc.ts            19 types (webviewDidLaunch, import/export, files, search, upsells, preview)

Each module declares a minimal Pick<ClineProvider, ...> limited to the members it actually uses.
The old dispatcher is untouched in this commit; the router that consumes these modules lands next.

No behavior change; cases are byte-for-byte the original statements relocated.

* refactor(webview): replace dispatcher switch with thin domain router (S2)

webviewMessageHandler.ts shrinks from a 4k-line switch to a ~170-line router:
it builds a Map<WebviewMessageType, handler> from each domain module's exported
MessageTypes set + handle<Domain>Messages function and delegates by message.type.
The exported signature (provider, message, marketplaceManager?) is unchanged, so the
boundary (ClineProvider.setWebviewMessageListener) and all spec files pass untouched.
Unknown types fall through to the same commented default as before.

The 4 no-explicit-any suppressions that moved to handlers/chat.ts in the scaffold commit
are now the only re-homed entry; webviewMessageHandler.ts is fully typed (0 any).

* refactor(webview): extract TaskHistoryService from ClineProvider (S3a)

Move task-history mutation, webview broadcast, debounced globalState
write-through, and recent-tasks caching into a focused TaskHistoryService
with narrow DI ports. ClineProvider keeps identical public method
signatures and delegates to the service. recentTasksCache stays on the
provider (owned through a port) so delegation flows and existing tests
that read/write the field keep working unchanged.

* refactor(webview): extract ProviderProfileService from ClineProvider (S3a)

Move provider-profile CRUD, activation, and sticky-profile persistence into
a focused ProviderProfileService with narrow DI ports. ClineProvider keeps
identical public method signatures and delegates. providerSettingsManager
is injected as a getter port (read at call time) so tests that replace the
field after construction keep working; updateTaskHistory is forwarded with
exact argument arity to preserve spy call signatures.

* refactor(webview): extract MarketplaceService from ClineProvider (S3a)

Move on-demand marketplace data fetching into a focused MarketplaceService
with narrow DI ports. The timeout warning is injected as a port (wired to
vscode.window.showWarningMessage in ClineProvider) so the service has no
direct vscode dependency and stays unit-testable. ClineProvider keeps the
same public fetchMarketplaceData signature and delegates.

* feat(core): extract TaskOrchestrator service for task lifecycle and delegation state machine

Adds src/core/services/TaskOrchestrator.ts owning the task lifecycle and
delegation/subtask state machine previously embedded in ClineProvider (S3b).

- TaskOrchestratorDeps: narrow DI ports (S3a pattern) bound to the provider at
  call time so spies (getState/getGlobalState/updateTaskHistory/getTaskWithId)
  and post-construction taskRegistry/taskScheduler swaps keep working.
- Moves createTask, createTaskWithHistoryItem, cancelTask/cancelTaskInternal,
  clearTask, resumeTask, addClineToStack, removeClineFromStack, evictCurrentTask,
  markDelegatedChildInterrupted, delegateParentAndOpenChild,
  reopenParentFromDelegation, abandonSubtask, and runDelegationTransition.
- Behavior preserved byte-for-byte (error messages, assertValidTransition order,
  instanceId guards, cancelledDelegationChildIds semantics, tool_result injection,
  flush/retry, log output).
- Adds focused unit tests at the narrowest layer (single-open invariant on
  createTask, cancelTask instanceId guard + rehydrate, delegation parent-metadata
  persistence + child scheduling, reopenParentFromDelegation tool_result injection,
  abandonSubtask orphan/transition logic).

* refactor(core): slim ClineProvider by delegating task orchestration to TaskOrchestrator

ClineProvider now keeps only webview lifecycle, state assembly, settings/misc,
and thin delegates. Every moved method remains a public method on ClineProvider
with an identical name/signature/return type that delegates to the orchestrator
via a lazily-cached static helper (ClineProvider.getTaskOrchestrator(this)), so:

- Delegation specs that invoke ClineProvider.prototype.<method>.call(fakeProvider)
  against plain `this` objects keep working (no reliance on the prototype chain).
- vi.spyOn(provider, getState|getGlobalState|updateTaskHistory|getTaskWithId) and
  post-construction provider.taskRegistry/taskScheduler swaps keep intercepting
  (all deps are closures read at call time).
- handleModeSwitch/showTaskWithId/performPreparationTasks/getTaskWithId stay on the
  provider and are consumed via narrow ports.

Moved methods: addClineToStack, removeClineFromStack, evictCurrentTask,
markDelegatedChildInterrupted, createTaskWithHistoryItem, createTask, cancelTask,
cancelTaskInternal, clearTask, resumeTask, delegateParentAndOpenChild,
reopenParentFromDelegation, abandonSubtask, runDelegationTransition.

Also removes now-unused imports and drops the ClineProvider eslint-suppression
count from 12 to 6 (no increase; no new suppressions).

* X1: add DOMPurify sanitizeHtml primitive for webview dangerouslySetInnerHTML sites

- Add dompurify dependency to webview-ui and a strict allowlist sanitizer
  (sanitizeHtml.ts) using an isolated DOMPurify instance (no global hooks).
- TerminalOutput: keep escapeXML pinned to true (exported const + regression
  test) and sanitize converter output as belt-and-suspenders.
- MermaidBlock: set mermaid securityLevel to strict (was loose) so labels are
  HTML-escaped at the source.
- MermaidButton: sanitize the copied SVG HTML before injecting into the zoom
  modal (child-node rendering would detach the live diagram).
- TaskItem: sanitize search-highlight HTML before injecting.
- Add sanitizeHtml.spec.ts asserting script/event-handler/javascript: removal,
  span+color preservation, and SVG path allow-listing with nested script
  stripping.

* X2: tighten HMR CSP and verify the local dev server is Vite before serving HMR HTML

- Remove the https://* wildcard from the dev-only HMR CSP (script-src,
  style-src, connect-src). script-src now allows only https://*.posthog.com
  (telemetry), the local Vite origins, and the nonce. 'unsafe-eval' is kept
  (required by Vite HMR/react-refresh) with a comment that it is dev-only and
  must never appear in getHtmlContent's production CSP.
- Harden the localhost probe: after the root reachability check, GET
  /@vite/client and require a 2xx whose body identifies Vite, otherwise fall
  back to getHtmlContent (production HTML). A rogue process on :5173 can no
  longer serve scripts to the webview. Keeps the .vite-port gate and the
  existing hmr_not_running error message.
- Extend ClineProvider.spec.ts with a dev-mode getHMRHtmlContent suite
  (scoped beforeEach/afterEach reset/restore the axios mock since the outer
  beforeEach's vi.clearAllMocks does not clear mockImplementationOnce queues):
  CSP has no bare https://* wildcard, and the vite-identity probe falls back to
  production HTML when /@vite/client is unreachable or not-Vite.

* docs: update changelog, debt log, readme and add typed-message-protocol ADR

* [Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test (Zoo-Code-Org#1074)

* test(e2e): drain delayed mock stream deterministically in subtask suite

The API-hang subtask fixture used aimock's flat latency, which applies
per SSE chunk and is never interrupted by client disconnects, so a
cancelled delayed stream stayed pending server-side for chunks x latency
and could flush into the next test's traffic.

- delay only the first chunk via streamingProfile.ttft so the pending
  window is exactly the shared SUBTASK_API_HANG_RESPONSE_LATENCY_MS
- anchor the post-test drain to the request's aimock journal timestamp
  and wait out only the remainder of that bounded window

* docs(e2e): clarify subtask drain invariants from code review

---------

Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>

* fix(router-provider): fetch model metadata before context management decisions (Zoo-Code-Org#1053)

* fix(router-provider): fetch model metadata before context management decisions

Router providers (zoo-gateway, kimi-code) that are auth-scoped skip the
model cache entirely. On a fresh handler instance getModel() falls back
to hardcoded defaults (e.g. 200k context window) because the real model
list has not been fetched yet. Context management runs before
createMessage() which is where fetchModel() normally happens, so
condensing/truncation decisions use the wrong context window.

Add ensureModelFetched() to RouterProvider that fetches once when the
instance model map is empty. Call it in Task before context management
so getModel() returns accurate metadata from the API.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(router-provider): single-flight ensureModelFetched and earlier call site

Make ensureModelFetched single-flight so concurrent callers share a
single in-flight fetch instead of firing duplicates. Move the call site
before the cachedStreamingModel snapshot so the model info is accurate
from the start of the streaming session, not just for context management.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(router-provider): address review feedback on fetch failures and double-fetch

Make fetchModel single-flight and short-circuit once models are loaded so
auth-scoped providers do not hit the models endpoint twice per request.
Catch ensureModelFetched failures in Task via safeEnsureModelFetched so a
metadata fetch error falls back to defaults instead of ending the task.
Add reject-then-recover coverage and Task tests for the new call sites.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(test): spy private addToApiConversationHistory via TaskTestAccess

vi.spyOn on the private method fails check-types; route it through the
existing test access cast like the other private helpers.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

* v3.76.0: Release Readiness & Architecture Program — typed message protocol, slim ClineProvider, upstream sync

- Merge upstream/main (7 commits: canonical provider identifiers, router-provider metadata, openai-codex Fast mode, service-tier refactor, e2e subtask fix)
- Preserve Zoo Gateway removal from the fork; drop upstream's zoo-gateway references/tests that conflict with removed provider
- Typed + runtime-validated webview message protocol (16 security-sensitive types)
- Domain-split webview dispatcher and slim ClineProvider (TaskOrchestrator, TaskHistoryService, ProviderProfileService, MarketplaceService)
- Semble one-dir EACCES fix, download-only binary, pre-installed mode description merge-fill
- CLI event-listener leak fix, legacy credential write path retired, vscode-shim logger wired
- Webview HTML sanitization (DOMPurify) + HMR CSP hardening
- Fix path-mentions double-escaping regression; remove blank Zoo Gateway test stubs
- Update CHANGELOG.md + README.md; bump version to 3.76.0

Closes: #98

Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>

* fix: remediate new CodeQL alerts (port validation, temp files, command race, path escaping)

Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>

* fix: remediate 100 pre-existing CodeQL code-scanning alerts

- js/file-system-race: drop fs.access pre-checks in favor of direct
  read-with-error-handling (McpHub, extract-text, ReadFileTool), use
  readdirSync withFileTypes (find-missing-i18n-key), and exclusive 'wx'
  writes (bootstrap.mjs)
- js/insecure-temporary-file: use fs.mkdtemp private dirs + 0600 modes
  (diagnosticsHandler, ShadowCheckpointService spec/service)
- js/remote-property-injection: validate property keys in ModesView and
  FileChangesPanel
- js/disabling-certificate-validation: expand justification comment for
  the debug-only TLS override (networkProxy)
- js/file-access-to-http / http-to-file-access: sanitize image
  references (image-generation), validate base64 image payloads
  (openai-native, openai-codex), validate image bytes before writing
  (GenerateImageTool), validate OAuth token response schema (qwen-code)
- js/indirect-command-line-injection: use spawnSync with args array +
  editor allowlist (install-vsix)
- js/log-injection: sanitize control chars in mock-server URL logs
- js/missing-origin-check: add isTrustedMessage origin/source validator
  and apply it across ~30 webview message handlers

Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>

* fix: remediate remaining CodeQL alerts flagged on the branch

- ReadFileTool.ts: open a single file handle and stat/read through it to
  eliminate the stat-then-read (TOCTOU) race in both the native and
  legacy read paths
- FileChangesPanel.tsx + ModesView.tsx: store webview-sourced content by
  Map key instead of object property to prevent prototype pollution
  (remote-property-injection) via untrusted paths/slugs
- mcp-oauth.test.ts: sanitize the HTTP method as well as the URL in the
  mock-server log (log-injection)
- qwen-code.ts: validate refresh_token as a string primitive before
  persisting credentials (network-data-to-file)
- apps/vscode-e2e/tsconfig.json: use non-deprecated moduleResolution
  (Node10 + ignoreDeprecations 5.0) and set explicit rootDir
- readFileTool.spec.ts: mock fs.open handle so tests exercise the new
  single-handle read path (no new eslint suppressions)

Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>

---------

Co-authored-by: Alexei Gubin <36731953+WebMad@users.noreply.github.com>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>
Co-authored-by: Ivan Ramadhan Arifin <111653938+ivanarifin@users.noreply.github.com>
Co-authored-by: zoomote[bot] <305051434+zoomote[bot]@users.noreply.github.com>
Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: James Mtendamema <59908268+JamesRobert20@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: hanneke-de-vries <dhanneke204@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR changes are ready and waiting for maintainer re-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants